Remove some occurrences of too-many-branches code smells#836
Conversation
refactor: extract helpers to reduce pylint complexity
…ibutes refactor: reduce instance attributes in migration classes
refactoring too-many-branches
|
Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset. In case there are security findings, they will be communicated to you as a comment inside the PR. Hope you’ll enjoy using Jit. Questions? Comments? Want to learn more? Get in touch with us. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Reviewed by Cursor Bugbot for commit 3e2a085. Configure here.
|
|
||
| encoded_obj = _apply_builtin_encoder(obj) | ||
| if encoded_obj is not None: | ||
| return encoded_obj |
There was a problem hiding this comment.
Encoders returning None are silently dropped
Medium Severity
_apply_custom_encoder and _apply_builtin_encoder both return None as a "not found" sentinel, but None is also a valid encoder return value. The if encoded_obj is not None checks in jsonable_encoder cause the code to incorrectly skip an encoder that legitimately returns None and fall through to _encode_fallback_object, which will likely raise a ValueError. The original code returned the encoder's result directly without any None check.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3e2a085. Configure here.
| if inner_types and inner_types[0] is str: | ||
| attrs = {"sortable": sortable} | ||
| return {"name": field_name, "type": "tag", "attrs": attrs} | ||
| return _get_container_field_def(field_name, field_type, field_info) |
There was a problem hiding this comment.
Container fields with non-string types silently excluded from schema
Medium Severity
_get_container_field_def returns None when the container's inner type isn't str, and _get_field_type returns that None directly instead of falling through to the default TAG field. In the original code, container types with non-string inner types would fall through to return {"name": field_name, "type": "tag"}. Now they are silently excluded from the RedisVL schema.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3e2a085. Configure here.
| ) | ||
|
|
||
| applied_count = 0 | ||
| applied_count, errors = await self._run_pending_migrations_with_monitoring( |
There was a problem hiding this comment.
Dead store: applied_count immediately overwritten
Low Severity
applied_count = 0 on line 347 is immediately overwritten by the tuple unpacking on line 348 (applied_count, errors = await self._run_pending_migrations_with_monitoring(...)). The initial assignment is a dead store and serves no purpose.
Reviewed by Cursor Bugbot for commit 3e2a085. Configure here.
2 participants
Note
Medium Risk
Large, behavior-preserving refactor across query execution, serialization, schema generation, and migrations; risk is regression in edge cases despite added tests, not security.
Overview
This PR refactors high-branch code across the library by splitting large functions into smaller helpers, without changing the intended public behavior. The goal is to satisfy static-analysis “too many branches” rules and make complex paths easier to maintain.
Serialization and encoding:
jsonable_encoderinencoders.pyis decomposed into typed helpers (Pydantic models, dicts, iterables, custom/builtin encoders, fallback). New tests cover nested types, custom encoders, andvars()fallback.Model core (
model.py): Timestamp/base64 conversion,FindQuery(internal_FindQueryState/_FindQueryCache, query building, JSON path projection,executepipeline), RediSearchresolve_value, deep-field validation,ModelMeta.__new__,HashModel.save(TTL preservation), JsonModel/HashModel schema builders,FieldInfo/VectorFieldOptions(optional params grouped inVectorFieldParameters), and related pieces are broken into focused functions.DefaultMetaswitches from a dataclass to class-levelClassVardefaults.Migrations: Datetime hash migration logic moves into
_collect_hash_keys,_process_hash_batch,_process_hash_key, etc., and drops redundantenable_resume/_processed_keystracking. Data and schema migrators gain helpers for running migrations, monitoring results, connection recovery, and rollback.Other:
render_treeand RedisVL field mapping use small helpers;legacy_migrator.detect_migrationsis simplified similarly.Tests added or extended for encoders, deep field paths, JSON path projection, render tree layout, timestamp conversion, and RedisVL schema field types.
Reviewed by Cursor Bugbot for commit 3e2a085. Bugbot is set up for automated code reviews on this repo. Configure here.